Skip to content

contours: validate input raster, reject complex dtype#2711

Merged
brendancol merged 1 commit into
xarray-contrib:mainfrom
brendancol:deep-sweep-security-contour-2026-05-29-01
May 31, 2026
Merged

contours: validate input raster, reject complex dtype#2711
brendancol merged 1 commit into
xarray-contrib:mainfrom
brendancol:deep-sweep-security-contour-2026-05-29-01

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2701

What

contours() did not validate its input before processing. This adds the standard _validate_raster() check the rest of the codebase uses.

  • A complex-dtype DataArray was silently coerced to float64 (imaginary part dropped, only a ComplexWarning), so contours were traced on the real part instead of being rejected. Now raises ValueError.
  • A non-DataArray input failed late inside ArrayTypeFunctionMapping with TypeError: Unsupported Array Type. Now raises a clear TypeError up front.

The inline ndim == 2 check is removed because _validate_raster(..., ndim=2) covers it with the same "must be 2D" message.

Backends

Validation runs before backend dispatch, so the single check covers numpy, cupy, dask+numpy, and dask+cupy. No backend-specific code changed.

Test plan

  • Added test for complex dtype rejection (ValueError)
  • Added test for non-DataArray rejection (TypeError)
  • Full test_contour.py suite passes (26 tests, including cupy and dask+cupy equivalence on a CUDA host)

…#2701)

contours() skipped the standard _validate_raster() check. A complex-dtype
DataArray was silently coerced to float64 (imaginary part dropped, only a
ComplexWarning), so contours were traced on the real part instead of being
rejected. A non-DataArray input failed late with a confusing dispatch error.

Add _validate_raster(agg, func_name='contours', name='agg', ndim=2) at the
top of contours(), matching curvature/morphology/sky_view_factor. This drops
the now-redundant inline ndim==2 check and adds tests for complex-dtype and
non-DataArray inputs.
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: contours: validate input raster, reject complex dtype

Blockers

None.

Suggestions

None.

Nits

  • test_non_dataarray_rejected matches on "xarray.DataArray", which depends on the exact wording of _validate_raster's TypeError message. Fine since both live in this repo, but the test breaks if that message ever changes. Low risk, no action needed.

What looks good

  • The fix matches the established pattern: curvature.py, morphology.py, and sky_view_factor.py call _validate_raster the same way.
  • Validation runs before backend dispatch, so the single check covers all four backends without touching backend code.
  • Removing the inline ndim == 2 check is correct. _validate_raster(..., ndim=2) raises a ValueError containing "2D", so test_invalid_ndim still passes.
  • Integer-dtype rasters still pass (numeric=True, integer_only=False), so no regression for the existing int input path.
  • Both new tests assert the right exception types: ValueError for complex, TypeError for non-DataArray.

Checklist

  • Algorithm unchanged (validation-only change)
  • All backends covered by the single pre-dispatch check
  • NaN handling unchanged
  • Edge cases (complex, non-DataArray, 3D, too-small) covered by tests
  • No premature materialization
  • No benchmark needed (validation guard)
  • README unchanged (no API surface change)
  • Docstring unchanged; the behavior change is input validation only

@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 29, 2026
@brendancol brendancol merged commit c7171d4 into xarray-contrib:main May 31, 2026
8 of 9 checks passed
Melissari1997 pushed a commit to Melissari1997/xarray-spatial that referenced this pull request May 31, 2026
…#2701) (xarray-contrib#2711)

contours() skipped the standard _validate_raster() check. A complex-dtype
DataArray was silently coerced to float64 (imaginary part dropped, only a
ComplexWarning), so contours were traced on the real part instead of being
rejected. A non-DataArray input failed late with a confusing dispatch error.

Add _validate_raster(agg, func_name='contours', name='agg', ndim=2) at the
top of contours(), matching curvature/morphology/sky_view_factor. This drops
the now-redundant inline ndim==2 check and adds tests for complex-dtype and
non-DataArray inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

contours() skips input validation: complex dtype silently coerced, non-DataArray errors late

1 participant